Skip to content

Prune redundant comments and document workaround refactors - #397

Open
ohld wants to merge 1 commit into
productionfrom
codex/prune-code-comments
Open

Prune redundant comments and document workaround refactors#397
ohld wants to merge 1 commit into
productionfrom
codex/prune-code-comments

Conversation

@ohld

@ohld ohld commented Nov 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Problem and change

Old disabled code, completed TODOs, and line-by-line narration obscure the explanations that matter. This removes a net 73 Python comment tokens across 17 files, with identical executable tokens and ASTs.

  • Delete disabled Sentry validation, Redis debug flush, old caption/keyboard/error branches and admin-check snippets.
  • Delete completed upload-object/confirmation TODOs and misleading callback labels; remove mechanical narration in watermarking, ETL, language insertion and OCR.
  • Preserve the multi-worker webhook rate-limit warning as a normal comment; clarify the popup first-click guard and watermark heuristic.
  • Correct the crossposting analytics rationale: ranker logging happens before sending, so best-effort handling prevents analytics from blocking a post. Preserve the database retry incident and timing while dropping the unsupported reliability claim.

High-level follow-up plan

These are proposals, not behavior changes in this PR, ranked by impact.

Priority Area and evidence Refactor and acceptance
1 Payout idempotency: src/tgbot/handlers/treasury/payments.py:pay_if_not_paid checks existence then inserts; the alert wrapper still says “TODO: atomic?”. The tracked treasury schema has no equivalent payout-key uniqueness constraint. Audit existing duplicate/non-null payout keys; enforce the applicable key in PostgreSQL and insert conflict-safely. Only the successful claimant should trigger notification. Test simultaneous claims, existing payments and notification failures. Preserve the warning until this lands.
2 Crosspost retry boundary: src/flows/crossposting/meme.py:post_meme_to_tgchannelen/post_meme_to_tgchannelru send to Telegram before receipt/status/reward bookkeeping. Separate essential delivery receipts from best-effort analytics/notifications and define recovery when Telegram accepts a post but the message-ID receipt fails to save. Merely moving code cannot make Telegram and PostgreSQL atomic. Test failures at each boundary and ensure retries cannot blindly republish. Keep the valid album retry warning in src/flows/rewards/uploaded_memes.py.
3 Reverse dependency: src/tgbot/repo/users.py:mark_user_blocked/mark_user_unblocked imports cache helpers lazily to avoid a cycle through user_info.py and service.py. Move block/unblock orchestration and best-effort cache refresh above the repository layer. Keep existing public entry points and test DB success plus cache failure. Remove “avoid cycle” comments only when the cycle is gone.
4 Repeated retry mechanics: src/database.py:fetch_one/fetch_all/execute/run_in_transaction. Consider one private retry/transaction mechanism only if it reduces complexity. Preserve read/write differences, budgets, backoff, return shapes and transaction lifetime; test mixed transient/deadlock sequences and exhaustion. Retain the incident rationale.

Keep

  • Telegram callback spinner, missing reaction markup, timezone/asyncpg and media constraints.
  • Redis lock ownership, cancellation-safe delivery recording, deduplication and source-status invariants.
  • OpenRouter free-only limits and cooldown/quota explanations.
  • Crossposting's PostgreSQL numeric/NULL quirks: the tracked quirk-fix A/B experiment remains running; resolving it is a product/experiment decision, not comment cleanup.
  • Tool directives, useful navigation, schema/analytics meaning and test intent.

The audit focused on application comments and workaround markers, with a targeted check of tests, scripts and migrations. It is deliberately conservative, not a blanket removal of comments or docstrings. No project-scoped GBrain source was available; findings use current repository evidence.

Validation

  • All 17 changed Python files have identical ASTs and executable token streams to the base.
  • Lint/type/coverage/format directives preserved.
  • ruff check src/ tests/, ruff format --check src/ tests/, redaction audit, single migration head, and git diff --check passed.
  • Full test suite runs in PR CI; no behavioral tests added for comment-only edits.
  • Independent GPT-6.1 Sol subagent review via the requested ask-matt router; its standalone code-review skill is unavailable locally, so Standards + Spec axes are applied directly.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@ohld